Skip to content

[Host.Outbox] Clean up in batches#376

Merged
zarusz merged 1 commit intozarusz:masterfrom
EtherZa:feature/375-outbox-clean-up-service
Mar 10, 2025
Merged

[Host.Outbox] Clean up in batches#376
zarusz merged 1 commit intozarusz:masterfrom
EtherZa:feature/375-outbox-clean-up-service

Conversation

@EtherZa
Copy link
Contributor

@EtherZa EtherZa commented Mar 4, 2025 •
edited
Loading

Changes:

  • Extracts OutboxCleanUpTask from OutboxSendingTask
  • Clean up routing occurs outside of send loop and on SMB start up
  • Clean up occurs in recurring batches until Outbox table has been cleaned
  • Clean up occurs with a low deadlock_priority so as to kill service instead of publishing queries
  • Include ROWLOCK, UPDLOCK, READPAST hints when locking messages
  • Include ROWLOCK, READPAST hints when removing messages
  • Excluded unused columns from db retrieval when selecting messages for publishing
  • Reworked the indexes

@EtherZa EtherZa force-pushed the feature/375-outbox-clean-up-service branch 4 times, most recently from 11b497a to 61bb13a Compare March 4, 2025 16:39
@EtherZa EtherZa changed the title [Host.Outbox] Clean up in batches [Host.Outbox] Clean up in batches [WIP] Mar 4, 2025
@zarusz
Copy link
Owner

zarusz commented Mar 4, 2025

Hi @EtherZa,

This is a valuable enhancement—great work!

Regarding the outbox integration tests, I wanted to share some context: our integration tests utilize a shared Azure SQL instance, meaning all test runs access the same database. Previously, when tests relied on the SQL Server container image provided by the build agent, we encountered significant unresponsiveness, leading to flaky outbox test results. I hope this insight is helpful. Please let me know if there's anything I can assist you with.​

@EtherZa EtherZa force-pushed the feature/375-outbox-clean-up-service branch 10 times, most recently from 140c58f to 0547739 Compare March 5, 2025 15:32
@EtherZa EtherZa changed the title [Host.Outbox] Clean up in batches [WIP] [Host.Outbox] Clean up in batches Mar 5, 2025
@EtherZa
Copy link
Contributor Author

EtherZa commented Mar 5, 2025

Hi @zarusz. That was quite the adventure but should hopefully be it. Please let me know your thoughts when you have a chance.

@EtherZa
Copy link
Contributor Author

EtherZa commented Mar 6, 2025

FYI: I'm reviewing the indexes. I think it can be better.

@zarusz
Copy link
Owner

zarusz commented Mar 6, 2025

Ok, cool let me know when to jump in for the review.

@EtherZa EtherZa force-pushed the feature/375-outbox-clean-up-service branch from 0547739 to 486988b Compare March 7, 2025 02:08
@EtherZa
Copy link
Contributor Author

EtherZa commented Mar 7, 2025

Thanks @zarusz. I'm done. Please review when you are able to.

zarusz
zarusz reviewed Mar 9, 2025
View reviewed changes
zarusz
zarusz reviewed Mar 9, 2025
View reviewed changes
src/SlimMessageBus.Host.Outbox/Services/OutboxCleanUpTask.cs Outdated

using Microsoft.Extensions.Hosting;

using SlimMessageBus;
Copy link
Owner

@zarusz zarusz Mar 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This using seems redundant

Copy link
Contributor Author

@EtherZa EtherZa Mar 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

zarusz
zarusz reviewed Mar 9, 2025
View reviewed changes
zarusz
zarusz reviewed Mar 9, 2025
View reviewed changes
src/Tests/SlimMessageBus.Host.Outbox.Test/SlimMessageBus.Host.Outbox.Test.csproj
<Import Project="../Host.Test.Properties.xml" />

<ItemGroup>
<ItemGroup>
Copy link
Owner

@zarusz zarusz Mar 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ident, reformat

Copy link
Contributor Author

@EtherZa EtherZa Mar 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@zarusz
Copy link
Owner

zarusz commented Mar 9, 2025

Nice work, @EtherZa! I’ve added a suggestion and a few minor comments - let me know what you think.

@EtherZa EtherZa force-pushed the feature/375-outbox-clean-up-service branch from 486988b to 5b9e3a5 Compare March 10, 2025 01:05
@EtherZa
zarusz#375 Outbox clean up in low priority batches with regular inter…
68925cc 
…vals

Signed-off-by: Richard Pringle <richardpringle@gmail.com>
@EtherZa EtherZa force-pushed the feature/375-outbox-clean-up-service branch from 5b9e3a5 to 68925cc Compare March 10, 2025 01:44
@sonarqubecloud
Copy link

sonarqubecloud bot commented Mar 10, 2025

Quality Gate Passed Quality Gate passed

Issues
2 New issues
0 Accepted issues

Measures
0 Security Hotspots
91.4% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@EtherZa
Copy link
Contributor Author

EtherZa commented Mar 10, 2025

Nice work, @EtherZa! I’ve added a suggestion and a few minor comments - let me know what you think.

Thanks. I have made the changes you requested and added some detail arounnd Task.Delay with TimeProvider. tldr; it's the same as System.Threading.Timer :)

@zarusz zarusz merged commit fff5361 into zarusz:master Mar 10, 2025
3 checks passed
@EtherZa
Copy link
Contributor Author

EtherZa commented Mar 18, 2025

Hi @zarusz. Do you have any idea when this may escape to nuget.org?

@zarusz
Copy link
Owner

zarusz commented Mar 20, 2025

Hey, I will try to push another minor release here soon. Sorry for the delay. I am out and skiing :)

@EtherZa
Copy link
Contributor Author

EtherZa commented Mar 20, 2025

That sounds like a superb reason! Have a fantastic time

@zarusz zarusz mentioned this pull request Mar 27, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@zarusz zarusz zarusz left review comments

Assignees

@EtherZa EtherZa

Labels

enhancement Host.Outbox Outbox.Sql

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@EtherZa @zarusz